[WIP] Refactor #225
Conversation
There was a problem hiding this comment.
Code Review
This pull request cleans up unused imports across several files, including removing time in manager.py, os in mixin.py, and various types in cli.py. It also removes from __future__ import annotations from cli.py and quotes the ConfigRegistry type annotation as a result. The review feedback points out that removing from __future__ import annotations will break compatibility with Python < 3.10 due to the use of PEP 604 union types and PEP 585 generic collections. It is recommended to restore this import, which also allows using ConfigRegistry directly without quotes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # Copyright (c) ModelScope Contributors. All rights reserved. | ||
| from __future__ import annotations | ||
|
|
||
| import os |
There was a problem hiding this comment.
Removing from __future__ import annotations will break compatibility with Python < 3.10 because of the use of PEP 604 union types (e.g., str | None) and PEP 585 generic collections (e.g., list[str]) in type annotations. Additionally, keeping it enables postponed evaluation of annotations, allowing forward references (like ConfigRegistry) to be used without enclosing them in quotes. Please keep this import.
| import os | |
| from __future__ import annotations | |
| import os |
| """Reads os.environ; recognizes TWINKLE_ prefix and any key known to the registry.""" | ||
|
|
||
| def __init__(self, registry: ConfigRegistry): | ||
| def __init__(self, registry: 'ConfigRegistry'): |
PR type
PR information
Write the detail information belongs to this PR.
Experiment results
Paste your experiment result here(if needed).